Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Hensel lift and zassenhaus #1066

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

nishnik
Copy link
Contributor

@nishnik nishnik commented Aug 17, 2016

It factors a primitive polynomial in integer domain.
Implements extended GCD in finite fields.
And a small bug fix in UIntPoly::mul

std::vector<UIntDict> zz_hensel_lift(const integer_class &p,
const std::vector<UIntDict> &f_list,
unsigned int l) const;
static void zz_divide(const UIntDict &a, const UIntDict &b,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you describe each of the methods above as a comment?

@certik
Copy link
Contributor

certik commented Aug 17, 2016

Otherwise it looks like it's working. Great job!

What needs to be done to finish this PR?

@nishnik
Copy link
Contributor Author

nishnik commented Aug 17, 2016

@certik I will have to write the factor square free method then this can be merged.
After which in a different PR, I would like to implement the get square free part and factor method.

s1 *= inv;
t0 = t1;
t1 = *t;
t1 *= inv;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is making copies too much. Can you reduce copying here?

@certik
Copy link
Contributor

certik commented Aug 17, 2016 via email

@nishnik
Copy link
Contributor Author

nishnik commented Aug 18, 2016

@certik Yes sure! I will try to complete it by this week.

*t = t0 - t1 * Q;

s0 = s1;
s1 = (*s) * inv;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what you basically want to do here is s0, s1 = s1, (s0 - s1 *Q) * inv.

How about this?

s0 = (s0 - s1 *Q) * inv;
swap(s0, s1);

std::vector<unsigned int> T_S(T.size());
auto it = std::set_difference(T.begin(), T.end(), S.begin(),
S.end(), T_S.begin());
T_S.resize(it - T_S.begin());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this line do?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants